-
Notifications
You must be signed in to change notification settings - Fork 151
[RFC-0011] OTEL integration based on alerts #1149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6e2dbbd to
7a793ec
Compare
261f89d to
95a19f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass
2721ea2 to
4d10e0d
Compare
|
All the uses cases with their respective screenshots were updated in the comment above. Most of them went as expected. However, about Additionally, if you see under the traces list, the naming is not really descriptive On the other hand, by checking the attribute limits (the information populated under every single span), looks like it's configurable (Attribute Limits). If it exceeds the limit set, it truncates the content, therefore I think the event message could be populated, as well. At this moment, I removed it from the attributes, could be easily added at any point in time |
|
Setting otel service name to We could also explore the option of sourcing the service name entirely from |
|
Resuming the discussion we had some time ago, this implementation does not support parent-child relationship across spans. Currently, the code forces a specific value for traceID (hash based on the convention: OTEL also allows to force spanID, however this one needs to be already present in the system (Jaeger or any other OTEL Collector), because that's the way to establish a parent-child relationship, refer to an existing spanID. Coming back to our use case, if we would like to have deeper level in our spans, we should think a way to establish that relationship. For instance and following the first use case above, have a Not necessarily needs to be tackled here in this PR, just to kick off the discussion a bit 😃. Any thoughts on this? @stefanprodan @matheuscscp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add otel to the provider docs and explain what it does
Sorry, I missed to add the documentation. Just added now 😄 . |
Signed-off-by: Adrian Fernandez De La Torre <[email protected]>
|
Hey @adri1197, I have one last question for this PR: The screenshots above are proving that after notification-controller has sent all the events of a particular |
Hi @matheuscscp! 😃 Yes, based on the use cases discussed. Perhaps, the top screenshots does not depict the whole information properly. I would suggest you if you can take a look at others shared below. All the spans you see on each screenshot belong to a unique trace (and therefore, they have a shared traceID - rootSpan). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @adri1197 🥇
Implementation of: fluxcd/flux2#5321
Closes: #1097
Use Cases
GitRepository->Kustomization->ConfigMapOCIRepository>HelmReleaseOCIRepository-
revision:6.9.1@sha256:565d310746f1fa4be7f93ba7965bb393153a2d57a15cfe5befc909b790a73f8aHelmRelease-
revision:6.9.1+565d310746f1-
oci-digest:sha256:565d310746f1fa4be7f93ba7965bb393153a2d57a15cfe5befc909b790a73f8a-
app-version:6.9.1HelmChart->HelmReleasesPart of: #1097
Part of: fluxcd/flux2#5321